Skip to content

Conversation

ochrons
Copy link
Contributor

@ochrons ochrons commented Nov 30, 2015

JS loading is deferred until after page render.

Note: .json compression must be enabled before merging this due to the large size of the search index.json

<a class="grey-text text-lighten-3" href="https://groups.google.com/forum/?fromgroups#!forum/scala-js">Mailing list</a>
<a class="grey-text text-lighten-3" href="https://groups.google.com/forum/?fromgroups#!forum/scala-js">Mailing list</a><br/>
<a class="grey-text text-lighten-3" href="http://twitter.com/scala_js">Twitter <i aria-hidden="true" class="fa fa-twitter"></i></a><br/>
<a href="http://github.com/scala-js/scala-js">GitHub <i aria-hidden="true" class="fa fa-github"></i></a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it should be in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is here because Twitter and Github links were removed from the top menu to make space for the search icon.

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

When I load the page, the search_data.json file is automatically downloaded. Could we make it so that it is only loaded when the user clicks the Search button?
That way, we avoid downloading that giant file for users on mobile that don't want to use Search at all (would often happen if they click on a link to a release news or something).

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

UI-wise, the interface is confusing. When I type in something, I have to switch to the mouse to go and click on a link. I would like to use up/down arrows to select things + Enter to go. Enter could default to the first item.

Also, the search "button" at the right of the input box appears as a link but does nothing. It should be removed if it doesn't do anything.

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

That's all, I think.

@ochrons
Copy link
Contributor Author

ochrons commented Feb 14, 2016

The search data download is deferred after the page is shown, so it doesn't hinder usage. It's also cached and gzipped, so the penalty is quite low. If it were downloaded only when the search button is clicked, it would be too late from UX point of view.

After gzip it's <100kB in size.

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

If it were downloaded only when the search button is clicked, it would be too late from UX point of view.

There's still the time to download it while the user types in something.

After gzip it's <100kB in size.

I guess that's acceptable, but it still feels like a big hit for all the mobile users who won't ever use the search box, if they hit it on Data usage.

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

Maybe I'm too paranoid, here.

@ochrons
Copy link
Contributor Author

ochrons commented Feb 14, 2016

The search button is indeed superfluous, but you can navigate with Tab+Enter. I'll make an update for this. Arrow key navigation would be nice, but probably requires a bit more work, so can do that as a separate update later.

On mobile the issue is latency, not bandwidth, so downloading search data JIT would not work that well.

Besides, comparing to the old scala-js.org site, I think the overall download size and request count is still way less :)

@sjrd
Copy link
Member

sjrd commented Feb 14, 2016

OK

@sjrd
Copy link
Member

sjrd commented Feb 27, 2016

That's all.

JS loading is deferred until after page render.

Note: .json compression must be enabled before merging this due to the large size of the search index.md
@sjrd
Copy link
Member

sjrd commented Feb 27, 2016

LGTM

sjrd added a commit that referenced this pull request Feb 27, 2016
Implemented local search with lunr.js
@sjrd sjrd merged commit 2590ca7 into scala-js:master Feb 27, 2016
@ochrons ochrons deleted the search branch February 27, 2016 12:00
@sjrd sjrd mentioned this pull request Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants